node:http2: match Node's unsupported-protocol check in connect() by robobun · Pull Request #43505 · oven-sh/bun

robobun · 2026-09-19T18:47:40Z

Problem

http2.connect("ftp://127.0.0.1:1") throws ERR_HTTP2_UNSUPPORTED_PROTOCOL with the message ftp:. Node.js v26.3.0 says protocol "ftp:" is unsupported. ErrorCode.cpp has no message text for this code, so the first argument becomes the message.
The ClientHttp2Session constructor (src/js/node/http2.ts) checks the protocol before it looks at options.createConnection. Node.js checks it only when it opens the socket itself (connect()). So http2.connect("ftp://host", { createConnection }) throws in Bun only.
request() takes the default :scheme from the URL, then from its own options. http2.connect({ port }, { protocol: "http:" }) sends :scheme: https. Node.js sends http.

Fix

Add the message row to the simpleErrorMessages table. Move the protocol switch into connectWithProtocol, which runs only when createConnection is not a function. This supersedes node:http2: add throw for ERR_HTTP2_UNSUPPORTED_PROTOCOL #17927, which has the same placement.
The constructor stores the default :scheme: the connect-time protocol without the trailing : (as in Node.js). request() reads it for both header forms (an object, a raw [name, value, ...] array). Without this, a custom protocol would send :scheme: ftp:.
A protocol in the request() options no longer changes :scheme. Node.js has no such option. The #url field had no other reader, so it is gone.
Verified: test/js/node/http2/node-http2-connect-protocol.test.ts (4 tests, a debug build of main fails all 4, Node.js passes the same scenarios). Also 14 upstream test-http2-* files. Self-reviewed: 5 concerns raised, 5 addressed (see Notes).

Background

http2.connect(authority, options) opens a client session. The protocol is authority.protocol, then options.protocol, then https:.
options.createConnection lets the caller supply the transport. The protocol then only names the :scheme.
$ERR_* calls in built-in JS go through jsFunctionMakeErrorWithCode. A code with no case there and no table row uses its first argument as the message.

Notes

Repro (run with bun file.cjs and node file.cjs):

const http2 = require("node:http2");
const net = require("node:net");
try { http2.connect("ftp://127.0.0.1:1"); } catch (e) { console.log(e.code, JSON.stringify(e.message)); }

const server = http2.createServer();
server.on("stream", (stream, headers) => { stream.respond(); stream.end(headers[":scheme"]); });
server.listen(0, "127.0.0.1", async () => {
  const port = server.address().port;
  const createConnection = () => net.connect(port, "127.0.0.1");
  const get = (client, requestOptions) => new Promise(resolve => {
    const req = client.request({ ":path": "/" }, requestOptions);
    let body = "";
    req.setEncoding("utf8").on("data", d => (body += d)).on("end", () => { client.close(); resolve(body); });
  });
  try {
    console.log(await get(http2.connect(`ftp://127.0.0.1:${port}`, { createConnection })));
  } catch (e) { console.log(e.code); }
  console.log(await get(http2.connect({ hostname: "127.0.0.1", port }, { protocol: "http:" })));
  console.log(await get(http2.connect({ hostname: "127.0.0.1", port }, { createConnection }), { protocol: "http:" }));
  server.close();
});

	Node.js v26.3.0	bun 1.4.3-canary.1+367d939d9	this PR
message	`protocol "ftp:" is unsupported.`	`ftp:`	`protocol "ftp:" is unsupported.`
`ftp://` with `createConnection`	`ftp`	throws `ERR_HTTP2_UNSUPPORTED_PROTOCOL`	`ftp`
`{ protocol: "http:" }` in the `connect()` options	`http`	`https`	`http`
`{ protocol: "http:" }` in the `request()` options, default `https:` session	`https`	`http`	`https`

The tests have their own file. They need nothing from node-http2.test.js, and alone they run in about 5 seconds on a debug build. node-http2.test.js takes over 6 minutes there, and several of its subprocess tests pass the 5 second limit on a debug build of main too.
Fail-before, measured the same way for both builds: bun bd test test/js/node/http2/node-http2-connect-protocol.test.ts with src/ at main gives 0 pass, 4 fail. With src/ at this branch it gives 4 pass.
node:http2: add throw for ERR_HTTP2_UNSUPPORTED_PROTOCOL #17927 (open since 2025-03, conflicts with main, targets src/bun.js/ paths that no longer exist) puts the throw in connectWithProtocol and adds the same message. Its review says "LGTM but needs a test". The throw at the top of the constructor came later, from compat(http2) validations and some behavior fixes #19558. This PR has the test.
builtins.d.ts gets the typed declaration, like the other codes whose message takes arguments.
The old request() code read options?.protocol by accident: it meant the connect() options, but options there is the second argument of request().
Not ported: with an object authority that has no port, Node.js builds the authority localhost:undefined. Bun keeps localhost:443.
The same missing-message cause affects ERR_HTTP_TRAILER_INVALID and ERR_HTTP_CONTENT_LENGTH_MISMATCH (node:http), ERR_INVALID_URL_SCHEME (url.fileURLToPath with the windows option) and ERR_SCRIPT_EXECUTION_INTERRUPTED (REPL). They are not in this PR on purpose: their call sites are in other modules, and one needs a change in NodeHTTPResponse.rs. node: give seven ERR_* codes Node's message text #43502 fixed them and is merged. This branch has main merged in, and the two changes do not conflict. node: fix broken error messages for ERR_HTTP2_UNSUPPORTED_PROTOCOL and 4 others #35791 and node:http: fix ERR_HTTP_CONTENT_LENGTH_MISMATCH and ERR_HTTP_TRAILER_INVALID message text #35777 (both closed unmerged) were earlier attempts.
node: fix broken error messages for ERR_HTTP2_UNSUPPORTED_PROTOCOL and 4 others #35791 also had this message fix. It was closed in a cleanup of stale pull requests, with no judgment on the fix.
Self-review: added the test for the request() options. requestJSON in the test no longer parses inside an event callback. This description now names what node:http2: add throw for ERR_HTTP2_UNSUPPORTED_PROTOCOL #17927 and node:http: fix ERR_HTTP_CONTENT_LENGTH_MISMATCH and ERR_HTTP_TRAILER_INVALID message text #35777 did, and the request() options change moved up into Fix. The sibling codes now point at node: give seven ERR_* codes Node's message text #43502.
Upstream files run with the debug build: test-http2-connect.js, test-http2-create-client-connect.js, test-http2-create-client-session.js, test-http2-raw-headers-defaults.js, test-http2-raw-headers.js, test-http2-sent-headers.js, test-http2-generic-streams.js, test-http2-connect-method.js, test-http2-connect-method-extended.js, test-http2-misused-pseudoheaders.js, test-http2-multiheaders-raw.js, test-http2-client-proxy-over-http2.js, test-http2-client-jsstream-destroy.js, test-http2-https-fallback.js. Also node-http2-client-close.test.ts, node-http2-continuation.test.ts, node-http2-settings-ack-ordering.test.ts and test/js/node/errors/error-code-messages.test.ts.

[human-review] gate passed · iteration 1 · 4 files touched

fails on main (without fix)

ASAN without fix: 4 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/node/http2/node-http2-connect-protocol.test.ts"
bun test v1.4.3 (367d939d9)

test/js/node/http2/node-http2-connect-protocol.test.ts:
69 |       name: "Error",
70 |       code: "ERR_HTTP2_UNSUPPORTED_PROTOCOL",
71 |       message: `protocol "${protocol}" is unsupported.`,
72 |     });
73 | 
74 |     expect(thrownBy("ftp://127.0.0.1:1")).toEqual(unsupported("ftp:"));
                                               ^
error: expect(received).toEqual(expected)

  {
    "code": "ERR_HTTP2_UNSUPPORTED_PROTOCOL",
-   "message": "protocol "ftp:" is unsupported.",
+   "message": "ftp:",
    "name": "Error",
  }

- Expected  - 1
+ Received  + 1

      at <anonymous> (/workspace/bun/test/js/node/http2/node-http2-connect-protocol.test.ts:74:43)
(fail) http2.connect() authority protocol > throws ERR_HTTP2_UNSUPPORTED_PROTOCOL with Node's message [95.13ms]
84 |   test("is not checked when options.createConnection opens the socket", async () => {
85 |     const { server, clientSide } = echoServerOverDuplexPair();
86 |     const createConnection
... (truncated)

release without fix: 4 FAILED
bun test v1.4.3-canary.1 (367d939d9)

test/js/node/http2/node-http2-connect-protocol.test.ts:
69 |       name: "Error",
70 |       code: "ERR_HTTP2_UNSUPPORTED_PROTOCOL",
71 |       message: `protocol "${protocol}" is unsupported.`,
72 |     });
73 | 
74 |     expect(thrownBy("ftp://127.0.0.1:1")).toEqual(unsupported("ftp:"));
                                               ^
error: expect(received).toEqual(expected)

  {
    "code": "ERR_HTTP2_UNSUPPORTED_PROTOCOL",
-   "message": "protocol "ftp:" is unsupported.",
+   "message": "ftp:",
    "name": "Error",
  }

- Expected  - 1
+ Received  + 1

      at <anonymous> (/workspace/bun/test/js/node/http2/node-http2-connect-protocol.test.ts:74:43)
(fail) http2.connect() authority protocol > throws ERR_HTTP2_UNSUPPORTED_PROTOCOL with Node's message [1.21ms]
84 |   test("is not checked when options.createConnection opens the socket", async () => {
85 |     const { server, clientSide } = echoServerOverDuplexPair();
86 |     const createConnection = mock((_authority: URL) => clientSide);
87 |     let client: http2.ClientHttp2Session | undefined;
88 |     try {
89 |       client = connect("ftp://example.test", { createConnect
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/node/http2/node-http2-connect-protocol.test.ts"
bun test v1.4.3 (367d939d9)

test/js/node/http2/node-http2-connect-protocol.test.ts:
(pass) http2.connect() authority protocol > throws ERR_HTTP2_UNSUPPORTED_PROTOCOL with Node's message [109.63ms]
(pass) http2.connect() authority protocol > is not checked when options.createConnection opens the socket [865.33ms]
(pass) http2.connect() authority protocol > from options.protocol is the default :scheme of a request [628.41ms]
(pass) http2.connect() authority protocol > is not overridden by a protocol in the request() options [131.16ms]

 4 pass
 0 fail
 10 expect() calls
Ran 4 tests across 1 file. [6.21s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 1021ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/127] gen ErrorCode+*.h
[2/127] gen cpp.rs (cppbind)
[3/127] gen JS modules (bundle-modules)
Preprocess modules (13334ms)
Bundle modules (119ms)
Postprocesss modules (27ms)
Bundle Functions (661ms)
Generate Code (43ms)

[14.19s] Bundled "src/js" for production
  2606 kb
  197 internal modules
  13 native modules
  50 internal functions across 16 files
[3/10] cargo bun_runtime → libbun_runtime.a
�[1m�[33mwarning�[0m�[1m: binary `bun_shim_impl` should have a kebab-case name�[0m
   �[1m�[94m|�[0m
�[1m�[94m 1�[0m �[1m�[94m|�[0m /workspace/bun/build/release/rust-target/.../bun_shim_impl
   �[1m�[94m|�[0m                                              �[1m�[33m^^^^^^^^^^^^^�[0m
   �[1m�[94m|�[0m
   �[1m�[94m= �[0m�[1mnote�[0m: `cargo::non_kebab_case_bins` is set to `warn` by default
�[1m�[96mhelp�[0m: to change the binary name to `bun-shim-impl`, convert `bin.name`
  �[1m�[94m--> �[0msrc/install/windows-shim/Cargo.toml:41:8
   �[1m�[94m|�[0m
�[1m�[94m41�[0m �[91m- �[0mname = �[91m"bun_shim_impl"�[0m
�[1m�[94m
... (truncated)

diff hotspot

src/js/builtins.d.ts                               |   1 +
 src/js/node/http2.ts                               |  43 +++----
 src/jsc/bindings/ErrorCode.cpp                     |   1 +
 .../node/http2/node-http2-connect-protocol.test.ts | 135 +++++++++++++++++++++
 4 files changed, 151 insertions(+), 29 deletions(-)

gate history · 2 passed · 1 rejected · iteration 1

evidence per changed file

file                                                    reads  edits  tests
src/js/builtins.d.ts                                        1      1     11
src/js/node/http2.ts                                        4      7     14
src/jsc/bindings/ErrorCode.cpp                              1      1     11
test/js/node/http2/node-http2-connect-protocol.test.ts      0      2      9

http2.connect() throws ERR_HTTP2_UNSUPPORTED_PROTOCOL with Node's message, and only when it opens the socket itself. With options.createConnection the protocol is not checked. request() takes its default :scheme from the protocol the session was connected with, minus the trailing colon. A protocol in the request() options no longer changes it. Co-authored-by: Meghan Denny <hello@nektro.net>

coderabbitai · 2026-09-19T18:47:46Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: d3e6d6d9-6194-4427-8a5b-8b92b19db5ac

📥 Commits

Reviewing files that changed from the base of the PR and between 42cc173 and ae43c6a.

📒 Files selected for processing (4)

src/js/builtins.d.ts
src/js/node/http2.ts
src/jsc/bindings/ErrorCode.cpp
test/js/node/http2/node-http2-connect-protocol.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Walkthrough

The change adds HTTP/2 unsupported-protocol error handling, restricts client connections to http: and https:, stores the selected default scheme, and adds protocol behavior tests.

Changes

HTTP/2 protocol handling

Layer / File(s)	Summary
Unsupported protocol error contract `src/js/builtins.d.ts`, `src/jsc/bindings/ErrorCode.cpp`	Adds the `$ERR_HTTP2_UNSUPPORTED_PROTOCOL` declaration and formats its runtime message as `protocol "<argument>" is unsupported.`
Client protocol selection `src/js/node/http2.ts`	Selects `net.connect` for `http:` and `tls.connect` for `https:`. Other protocols raise `ERR_HTTP2_UNSUPPORTED_PROTOCOL`. The client session stores the protocol-derived default scheme without its trailing colon.
Request scheme propagation `src/js/node/http2.ts`, `test/js/node/http2/node-http2-connect-protocol.test.ts`	Uses the stored default scheme for raw and object-form requests. Tests cover unsupported protocols, custom sockets, authority protocols, and request-level protocol options.

Suggested reviewers: cirospaciari

Priority: ⬇️ Low

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the primary change: matching Node.js unsupported-protocol behavior in node:http2 connect().
Description check	✅ Passed	The description explains the problem, fix, scope, background, and verification results. It does not use the exact template headings, but it provides the required information in equivalent sections.

_{Comment @coderabbitai help to get the list of available commands.}

robobun · 2026-09-19T18:47:51Z

^{Updated 12:18 AM PT - Sep 20th, 2026}

✅ @robobun, your commit ae43c6a19bd0b5f36f2058cf00f8b32d2e1c5fc0 passed in Build #118855! 🎉

🧪 To try this PR locally:

bunx bun-pr 43505

That installs a local version of the PR into your bun-43505 executable, so you can run:

bun-43505 --bun

claude

I reviewed this PR and didn't find any bugs. Because it changes observable node:http2 behavior (the throw now happens only when http2 opens the socket itself, and a request-level protocol option no longer affects :scheme) and touches src/js/builtins.d.ts, which has a designated code owner, a human look is still worthwhile.

What was reviewed:

Compared the new connectWithProtocol switch and constructor ordering against Node v26.3.0 core.js connect(): the check sits in the non-createConnection branch there too, and the protocol "<p>" is unsupported. text matches Node's ERR_HTTP2_UNSUPPORTED_PROTOCOL message; the simpleErrorMessages row takes 1 argument, matching the single call site and the new .d.ts declaration.
Confirmed both request paths (object headers and raw [name, value, ...] array) now read the same #defaultScheme, and that #url had no remaining readers after the removal.
Checked that nothing between the old throw site and the new one (authority string, onConnect closure) acquires a socket or timer, so the later throw does not leak a resource; #defaultScheme without an initializer matches the neighboring #authority/#socket_proxy fields.
The only other suite assertion on this code (test/js/node/test/parallel/test-http2-connect.js:127) checks code and name only, so it is unaffected by the message change.

Extended reasoning...

Overview

The PR touches four files. src/js/node/http2.ts moves the unsupported-protocol throw from the top of the ClientHttp2Session constructor into connectWithProtocol (reached only when options.createConnection is not a function), replaces the stored #url with a precomputed #defaultScheme (the connect-time protocol minus its trailing colon, via a newly hoisted StringPrototypeSlice primordial), and makes both request() header paths use that field instead of re-deriving the scheme from this.#url.protocol || options?.protocol where options was accidentally the per-request options. src/jsc/bindings/ErrorCode.cpp adds a simpleErrorMessages row so $ERR_HTTP2_UNSUPPORTED_PROTOCOL(protocol) produces Node's message instead of echoing the argument; src/js/builtins.d.ts adds the matching typed declaration. The test file adds a describe block with four tests covering the throw (string/URL/object authority, options.protocol, non-function createConnection), the createConnection bypass with :scheme: ftp observed server-side over a duplexPair, options.protocol: "http:" becoming the default scheme, and a request-level protocol option not overriding the session scheme.

Security risks

None identified. The relaxed check only applies when the caller supplies their own transport via createConnection, in which case the protocol string is only used as the :scheme pseudo-header value; this mirrors Node exactly. No user input reaches native code differently than before, and the C++ change is a constexpr table entry with a fixed argument count of 1 that matches the single call site.

Level of scrutiny

Moderate. The diff is small and closely tracks Node's connect() in lib/internal/http2/core.js, which I compared against directly: Node computes protocol = authority.protocol || options.protocol || 'https:', then only reaches its switch (protocol) (with default: throw new ERR_HTTP2_UNSUPPORTED_PROTOCOL(protocol)) in the else branch of typeof options.createConnection === 'function', and request() sets :scheme from session[kProtocol].slice(0, -1). The refactor removed two divergent copy-pasted scheme derivations in favor of one field, which is the right direction. I verified that the code path between the old throw location and the new one allocates no socket or timer, so a throw from connectWithProtocol does not leak anything the earlier throw would have avoided. The mock/duplexPair helpers used by the tests are already imported in the file. Still, this is an observable behavior change in a node:* module (two inputs that previously threw or sent a different scheme now behave differently), and src/js/builtins.d.ts matches a *.d.ts CODEOWNERS entry, so a human sign-off is appropriate rather than an automated approve.

Other factors

The four candidate issues raised during the hunt were all Node-parity consequences rather than defects: the request-level protocol override and the createConnection bypass are exactly Node's semantics, the uninitialized #defaultScheme field matches sibling private fields in the same class, and non-string protocol values are coerced the same way Node's slice call coerces them. The only pre-existing suite assertion on this error (test-http2-connect.js) checks code and name, not message, so no existing test was weakened. One pre-existing divergence not introduced here: Bun defaults the port using the merged protocol, while Node uses only authority.protocol; the PR does not claim to address that.

robobun · 2026-09-19T18:56:50Z

Status

Reproduced on bun 1.4.3-canary.1+367d939d9 against Node.js v26.3.0 with the script in the description (bun file.cjs, then node file.cjs). The outputs differ in four places: the message of ERR_HTTP2_UNSUPPORTED_PROTOCOL, the throw with createConnection, the default :scheme from the connect() options, and the protocol request option.

The four tests in test/js/node/http2/node-http2-connect-protocol.test.ts fail on a debug build of main and pass on a debug build of this branch:

bun bd test test/js/node/http2/node-http2-connect-protocol.test.ts   # src/ at main:        0 pass, 4 fail
bun bd test test/js/node/http2/node-http2-connect-protocol.test.ts   # src/ at this branch: 4 pass, 0 fail

PR: #43505

The tests do not depend on anything in node-http2.test.js. On their own they run in about 5 seconds on a debug build.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

claude

Code review found no issues

No high-confidence issues detected in this change.

claude

Code review found no issues

No high-confidence issues detected in this change.

robobun requested a review from alii as a code owner September 19, 2026 18:47

github-actions Bot added the claude label Sep 19, 2026

claude Bot reviewed Sep 19, 2026

View reviewed changes

node:http2: move the connect() protocol tests to their own file

9399b26

The tests do not depend on anything in node-http2.test.js. On their own they run in about 5 seconds on a debug build.

github-actions Bot reviewed Sep 19, 2026

View reviewed changes

Comment thread src/js/node/http2.ts Outdated

robobun mentioned this pull request Sep 19, 2026

node: fix broken error messages for ERR_HTTP2_UNSUPPORTED_PROTOCOL and 4 others #35791

Closed

claude Bot reviewed Sep 19, 2026

View reviewed changes

Comment thread test/js/node/http2/node-http2-connect-protocol.test.ts

node:http2: put the node reference for the default :scheme on one line

2cf1737

claude Bot reviewed Sep 19, 2026

View reviewed changes

robobun mentioned this pull request Sep 20, 2026

node:http2: add throw for ERR_HTTP2_UNSUPPORTED_PROTOCOL #17927

Closed

Merge branch 'main' into robobun/d80742c6/http2-unsupported-protocol

ae43c6a

claude Bot reviewed Sep 20, 2026

View reviewed changes

Navigation Menu

Repository navigation

node:http2: match Node's unsupported-protocol check in connect() - #43505

node:http2: match Node's unsupported-protocol check in connect()#43505
robobun wants to merge 4 commits into
mainfrom
robobun/d80742c6/http2-unsupported-protocol

robobun commented Sep 19, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 19, 2026 •

edited

Loading

Uh oh!

robobun commented Sep 19, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Sep 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robobun commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Background

Uh oh!

coderabbitai Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

robobun commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Sep 19, 2026 •

edited

Loading

coderabbitai Bot commented Sep 19, 2026 •

edited

Loading

robobun commented Sep 19, 2026 •

edited

Loading

robobun commented Sep 19, 2026 •

edited

Loading